Every other function type has a FunctionOption field around here. Do we need that for table functions as well?

message TableFunctionRel {
    ...
    repeated FunctionOption options = 4;
    ...
}

For the sake of consistency, maybe it is worth it? I don't understand what this FunctionOption is for... (code archaeology time!)

I think that these options are actually these bits of the yaml:

%YAML 1.2
---
urn: extension:io.substrait:functions_logarithmic
scalar_functions:
  -
    name: "ln"
    description: "Natural logarithm of the value"
    impls:
      - args:
          - name: x
            value: i64
        options: # all of these parts below
          rounding:
            values: [ TIE_TO_EVEN, TIE_AWAY_FROM_ZERO, TRUNCATE, CEILING, FLOOR ]
          on_domain_error:
            values: [ NAN, "NULL", ERROR ]
          on_log_zero:
            values: [NAN, ERROR, MINUS_INFINITY]
        return: fp64

So given that, it would make sense to include them.

Would it be better to come up with a more explicit way to mark the inability of deriving an output schema for a particular table function? Or is it sufficient to just let the absence of the schema field indicate this?

Is a more appropriate name for this GeneratorTableFunctionRel as introduced by
@jacques-n here?

In my mind there are two possible paths we can go down which determine the appropriate name.

1. We could explore expanding this proto (by e.g. adding a relations input field) so that it appropriately models both kinds of table functions. In this case, keeping the name as TableFunctionRel makes sense.
2. We could introduce a brand new relation to represent table functions which take relations as input. In this case, it would make sense to rename this relation to GeneratorTableRel or some name which distinguishes it from the other kind of table function.

From my biased view, it is better to categorize them... roughly

• Scan like (0 relational input)
• Project or Filter like (1 relational input)
• (N-ary) Join like (2+ relational inputs optional grouping support)
• Group like (1 relational input with grouping support)

This gives way more modular and systematic way to describe a table function.

Is your suggestion to have a different relation for each one of those? Or one shared relation? Maybe something like the following would be preferable...

message TableFunctionRel {
  ...
  oneof {
    ScanLike scan_like = 1; // PR implements this one
    //ProjectLike project_like = 2;
    //JoinLike join_like = 3;
    //GroupLike group_like = 4;
  }
  // implementations below
  ...
}

I prefer Rel for each type, clearly expressing the degree of input at message level. But I'm open to exploring oneof definition.

I added this to be consistent with the other function definitions, though AFAICT, this isn't really used anywhere. I found #193 from a few years ago which suggests that this should continue to be updated, but I'm not sure if that is still the prevailing philosophy.

Other functions have variadic final argument logic like:

message TableFunction {
    ...
    oneof final_variable_behavior {
      FinalArgVariadic variadic = 10;
      FinalArgNormal normal = 11;
    }
    ...
}

However, I skipped it for now for simplicity. Is it necessary on a first pass?

Because you have unnest, I think it is actually makes sense to have variadic support. 😆

I like the idea of introducing variadic argument handling, but as I understand it, it is a bit complicated in the current state to introduce variadics for things like unnest, since each argument of unnest could potentially be a different list type, e.g. unnest(ARRAY[1,2,3], ARRAY["a","b","c"], ARRAY[true,true,false]).

So unlike the typical usage of variadic arguments for other function types, where the last argument is the same type, this usage is a bit more complicated and I'm not sure yet how it could be encoded. An approach is to instead keep the implementation as non-variadic and then introduce another tool to stitch relations together (something like a zip relation).

Indeed above case requires implicit type coercion rule which Substrait chose not to. So strictly speaking, your example is an invalid Substrait -- the user must have put cast (cough).

Ah sorry I think I misunderstood the point you were making initially. I thought it was about wanting to call unnest across an arbitrary number of arrays all of potentially different types (e.g. list<X>, list<Y>, list<Z>, $X\neq Y \neq Z$) but it sounds like instead you mean calling unnest on arbitrary of lists, all with the same type.

If that is the case, then use it makes sense to me to include variadic handling.

Postgresql does have some handling for unnest across different list types, but they make a point in the documentation to say that this is syntactic sugar:

The special table function UNNEST may be called with any number of array parameters, and it returns a corresponding number of columns, as if UNNEST (Section 9.19) had been called on each parameter separately and combined using the ROWS FROM construct.

Ideally, we would be able to model something like:

SELECT unnest(col) FROM table;

However, my understanding is that this is really syntactic sugar for:

SELECT u.value 
FROM table t 
CROSS JOIN LATERAL unnest(t.col) AS u(value);

If that is correct, then we really need LateralJoinRel to properly model this usage of unnest.

It is probably worth then clarifying somewhere in the documentation introduced in this PR that the arguments to a table function can't be field references.

I don't understand SELECT unnest(col) FROM table. What is the type of that statement?

Below makes sense.

SELECT LIST(UNNEST(col)) FROM table

If you want to flatten all the lists in the table.col, then your lateral join is way to go. "Table" can only appear in the FROM clause in SQL (I believe 😄 ).

At least in postgresql, SELECT unnest(col) FROM table; is legal (see example here). This is documented here.

Functions returning sets can also be called in the select list of a query. For each row that the query generates by itself, the set-returning function is invoked, and an output row is generated for each element of the function's result set.

and also

PostgreSQL's behavior for a set-returning function in a query's select list is almost exactly the same as if the set-returning function had been written in a LATERAL FROM-clause item instead. For example,

SELECT x, generate_series(1,5) AS g FROM tab;

is almost equivalent to

SELECT x, g FROM tab, LATERAL generate_series(1,5) AS g;

It would be exactly the same, except that in this specific example, the planner could choose to put g on the outside of the nested-loop join, since g has no actual lateral dependency on tab. That would result in a different output row order. Set-returning functions in the select list are always evaluated as though they are on the inside of a nested-loop join with the rest of the FROM clause, so that the function(s) are run to completion before the next row from the FROM clause is considered.

Though of course, postgresql is just one DB in a sea of DBs 😅

Hmm... I see. Looks like PostgreSQL chose to allow the syntactic sugar (to me) which I disagree to have but oh well. 😆

TODO: this could probably use a little work to make it a bit nicer. I'll work on that later once we have some general agreement on the spec changes.

The change here is for now just to get CI/CD to pass.

TODO: I think this section could use a little bit of work:

• add some examples with YAML files to demonstrate the difference between derived / explicit
• Make the fact that there are these two kinds a bit clearer.

From my biased view, it is better to categorize them... roughly

• Scan like (0 relational input)
• Project or Filter like (1 relational input)
• (N-ary) Join like (2+ relational inputs optional grouping support)
• Group like (1 relational input with grouping support)

This gives way more modular and systematic way to describe a table function.

Because you have unnest, I think it is actually makes sense to have variadic support. 😆

For the sake of consistency, maybe it is worth it? I don't understand what this FunctionOption is for... (code archaeology time!)

perhaps we can follow other extensions. for instance, in decimal rounding functions, we just write type rather than wrap it in a quote (i.e., decimal<P,S> rather than "decimal<P,S>").

Also, I'm wondering whether we can follow the same pattern as others (i.e., allow type expression).

Ah yes, I am operating under the assumption that we will indeed need to allow type expressions. I altered the functions.proto to accept a DerivationExpression.ExpressionNamedStruct. I'm not entirely sure, but I believe this is meant to capture type expressions.

Can we simply have table_schema only? If it is empty, it must be from the YAML and should be able to be derived. Otherwise, we can just use the table_schema. If both are present, we'd better validate.

I wanted to be consistent with the behavior of ScalarFunctions, which includes return types in the plans even though it is known from yaml files:

  message ScalarFunction {
    ...
    // Must be set to the return type of the function, exactly as derived
    // using the declaration in the extension.
    Type output_type = 3;
    ...
  }

I figured that it makes the plans more explicit to always include the table_schema, regardless of if things were derived or not. However, I still could drop the derived field. Then we just specify that the schema must conform to the implied schema in the yaml, if one is present. Otherwise, it can be anything set by the producer. What do you think?

It is fine to have table_schema always but there is consequence -- it can be large especially with names. So I prefer to be able to omit the table_schema so that let consumer derive it from YAML. This is another point to add to dialect whether the implementation supports YAML based derivation. If the implementation does not support derivation, the producer must send explicit table_schema.

Is this constant defined somewhere? What does it mean?

https://substrait.io/expressions/scalar_functions/#argument-types

Value arguments: arguments that refer to a data value. These could be constants (literal expressions defined in the plan) or variables (a reference expression that references data being processed by the plan).

My understanding is that constant means that a value can only be a literal and not a reference to a value unknown at plan construction e.g. a column reference. Although I'm unsure why we wouldn't allow these values to be expressions which lack references.

All that being said, postgresql does in fact accept column references in the arguments to generate_series (example here), so I think it makes sense to lift this restriction.

Even step size can be dynamic from table data!

We shouldn't unnecessarily restrict ourselves to literals and this is another place we shouldn't IMO. Implementation may not support generic expression in certain places but we shouldn't prevent it from the spec. In fact, this is another potential information to encode in dialect whether an expression has some restrictions.

Looks like there is an interesting spilit between stop being inclusive and exclusive.

• python, pandas and snowflake exclude the stop value.
• PostgreSQL, T-SQL, ZetaSQL include the stop value.

I guess either way is fine... Another problem is that some systems do support generating series for other numeric types (e.g., fp32, fp64, decimal) but we can add those later.

We could add this as a function option which specifies if they include or exclude the stop value. What do others think?

Do we want to say something about the default value of step?

Would it be better to instead drop the implementations where step is missing? That way, we force the plans to be more explicit.

I am wondering whether we can omit derived. Always having table_schema is fine and make it self-contained to some degree. Then, derived=true is implied and meaningful if and only if the engine is capable of handling YAML and YAML entry has struct there.

I'm okay with omitting derived. I included it initially because I thought it made the dependency a little clearer between the YAML and the plan. But ultimately, libraries implementing the substrait spec will have to check consistency between the table_struct and the yaml anyways, so it doesn't matter that much.

Leaving this conversation unresolved if others have something to say, but my most recent commit has dropped derived from the proto. Thanks!

I don't understand SELECT unnest(col) FROM table. What is the type of that statement?

Below makes sense.

SELECT LIST(UNNEST(col)) FROM table

If you want to flatten all the lists in the table.col, then your lateral join is way to go. "Table" can only appear in the FROM clause in SQL (I believe 😄 ).

Indeed above case requires implicit type coercion rule which Substrait chose not to. So strictly speaking, your example is an invalid Substrait -- the user must have put cast (cough).

I prefer Rel for each type, clearly expressing the degree of input at message level. But I'm open to exploring oneof definition.

I have made this a draft PR again. I need to have a proper think about how we can represent this dynamic return type if we really want to have these functions be variadic 😰